Skip to content

chore: refactoring and cleaning up the code to adhere to standards - #105

Merged
zhravan merged 10 commits into
mainfrom
trigger-b
Oct 1, 2025
Merged

chore: refactoring and cleaning up the code to adhere to standards#105
zhravan merged 10 commits into
mainfrom
trigger-b

Conversation

@zhravan

@zhravan zhravan commented Oct 1, 2025

Copy link
Copy Markdown
Owner

Summary

Describe the change and its motivation.

Checklist

  • Tests pass: make verify or golearn verify <slug>
  • Docs updated (README/CONTRIBUTING) if needed
  • No large new dependencies

Screenshots / Output (if CLI UX)

Paste before/after where helpful.

Related issues

Fixes #

Summary by CodeRabbit

  • New Features

    • Added JSON serialization/deserialization helpers for a Person object.
  • Chores

    • Added CI workflow enforcing formatting, static checks, build and comprehensive tests.
    • Added Pages deployment workflow to build and deploy the docs site.
  • Documentation

    • Added full docs site: start guide, exercises index, contributing, about, layouts, and README.
  • Style

    • Added Gopher-themed site CSS and theme configuration.
  • Tests

    • Removed obsolete XML-related tests.

@coderabbitai

coderabbitai Bot commented Oct 1, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@zhravan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 9 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 7894c5f and a339308.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)

Walkthrough

Adds a GitHub Actions CI workflow and a Pages deployment workflow; introduces a JSON exercise solution (Person with marshal/unmarshal helpers); removes an XML exercise test suite; and adds a Jekyll-based docs site (config, layouts, pages, theme CSS, Gemfile, contributing/getting-started materials).

Changes

Cohort / File(s) Summary
CI Workflow
.github/workflows/ci.yml
New GitHub Actions workflow triggered on pushes/PRs to main and develop: checkout, setup Go 1.22, cache modules, go vet, enforce gofmt, build bin/golearn, run CLI validations, run unit/exercise tests, per-exercise verification (with/without solutions), run binary from different dir, test watch command with timeout.
Pages Workflow
.github/workflows/pages.yml
New GitHub Actions workflow to build and deploy a Jekyll site to GitHub Pages: setup Ruby, bundle install, build site from docs, upload artifact, and deploy via actions/deploy-pages@v4.
JSON Exercise Solution
internal/exercises/solutions/36_json/json.go
Adds Person struct (name, email) and helpers MarshalPerson(Person) (string, error) and UnmarshalPerson(string) (Person, error) using encoding/json, propagating errors.
XML Tests Removal
internal/exercises/solutions/37_xml/xml_test.go
Removes the XML test suite (tests for Person/Book marshal/unmarshal and round-trip checks).
Documentation Site (Jekyll)
docs/* <br>docs/_layouts/*
docs/assets/css/gopher-theme.css <br>docs/README.md
docs/*.md <br>docs/_config.yml
``docs/Gemfile`
Adds a complete documentation site: site config, layouts (default.html, exercise.html), theme CSS, Gemfile, README, index/getting-started/exercises/about/contributing pages and other docs content for local dev, structure, and deployment.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub
  participant CI as CI Workflow
  participant Go as Go Toolchain

  Dev->>GH: Push / PR to main or develop
  GH-->>CI: Trigger CI workflow
  rect rgb(235,245,255)
    note right of CI: Setup & deps
    CI->>CI: Checkout repo
    CI->>Go: Setup Go 1.22
    CI->>CI: Cache modules / go mod download
  end
  rect rgb(245,255,235)
    note right of CI: Static checks
    CI->>Go: go vet ./...
    CI->>Go: gofmt -l (fail on diff)
  end
  rect rgb(255,245,235)
    note right of CI: Build & test
    CI->>Go: go build -o bin/golearn
    CI->>CI: Run CLI validations & per-exercise verify
    CI->>Go: go test ./...
  end
  CI-->>GH: Report status
Loading
sequenceDiagram
  autonumber
  actor Maintainer as Maintainer
  participant GH as GitHub
  participant PagesCI as Pages Workflow
  participant Ruby as Ruby/Bundler

  Maintainer->>GH: Push to main / workflow_dispatch
  GH-->>PagesCI: Trigger Pages workflow
  rect rgb(235,245,255)
    PagesCI->>PagesCI: Checkout
    PagesCI->>Ruby: Setup Ruby 3.1 & bundle install (docs/)
    PagesCI->>PagesCI: Build Jekyll site (docs/_site)
    PagesCI->>GH: Upload Pages artifact
  end
  rect rgb(245,255,235)
    PagesCI->>GH: Deploy via actions/deploy-pages@v4
    GH-->>PagesCI: Return deployed page URL
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I twitch my ears at CI’s chime,
Green carrots line up, right on time.
A Person JSON, neat and bright,
XML hops off into the night.
I thump my paw — docs bloom in light.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current title describes a code refactoring chore focused on cleaning up existing code, but the pull request actually introduces substantial new functionality—CI and Pages workflows, a Jekyll documentation site, JSON serialization helpers, and more—which is misleading and does not align with the main changes. Please update the title to concisely reflect the primary scope of this PR, for example: “feat: add GitHub Actions CI and Pages workflows and Jekyll documentation site” or split out refactoring into separate PRs so the title matches the actual changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d451ad and f14cc8d.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (1 hunks)
  • internal/exercises/solutions/36_json/json.go (1 hunks)
  • internal/exercises/solutions/37_xml/xml_test.go (0 hunks)
💤 Files with no reviewable changes (1)
  • internal/exercises/solutions/37_xml/xml_test.go
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml

18-18: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


23-23: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🔇 Additional comments (5)
.github/workflows/ci.yml (2)

1-133: Well-structured CI workflow with comprehensive coverage.

The workflow provides thorough testing including formatting checks, vetting, build verification, command testing, and exercise-specific validations. The use of caching and the variety of test scenarios (error handling, different directories, timeouts) demonstrate good CI practices.


76-79: No action needed
internal/exercises/solutions/37_xml/xml.go is present and the corresponding xml_test.go still exists under internal/exercises/templates/37_xml/, so the ./bin/golearn verify 37_xml --solution step remains valid.

internal/exercises/solutions/36_json/json.go (3)

13-16: Well-defined Person struct with proper JSON tags.

The struct correctly uses json struct tags for marshaling/unmarshaling, following Go conventions.


19-25: MarshalPerson correctly implements JSON encoding.

Error handling is appropriate, and the conversion from bytes to string is the standard approach.


28-35: UnmarshalPerson correctly implements JSON decoding.

The function properly converts the string to bytes, unmarshals into a Person struct, and returns a zero-value Person on error—a sensible Go idiom.

Comment thread .github/workflows/ci.yml
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update to setup-go@v5.

The actions/setup-go@v4 runner is deprecated and too old for GitHub Actions.

Apply this diff:

-      uses: actions/setup-go@v4
+      uses: actions/setup-go@v5

Based on static analysis hints.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uses: actions/setup-go@v4
uses: actions/setup-go@v5
🧰 Tools
🪛 actionlint (1.7.7)

18-18: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🤖 Prompt for AI Agents
.github/workflows/ci.yml around line 18: the workflow uses the deprecated
actions/setup-go@v4; update the action reference to actions/setup-go@v5 by
replacing the uses line so the workflow runs the supported v5 runner (ensure any
v4-specific inputs are compatible with v5 and adjust inputs if needed).

Comment thread .github/workflows/ci.yml
go-version: '1.22'

- name: Cache Go modules
uses: actions/cache@v3

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update to cache@v4.

The actions/cache@v3 runner is deprecated and too old for GitHub Actions.

Apply this diff:

-      uses: actions/cache@v3
+      uses: actions/cache@v4

Based on static analysis hints.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uses: actions/cache@v3
uses: actions/cache@v4
🧰 Tools
🪛 actionlint (1.7.7)

23-23: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🤖 Prompt for AI Agents
In .github/workflows/ci.yml around line 23, the workflow references the
deprecated actions/cache@v3; update that step to use actions/cache@v4 by
changing the uses entry to actions/cache@v4 (no other semantic changes
expected), then run a quick workflow lint or push to a branch to verify the
workflow runs correctly in CI.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)

18-18: Bump to actions/setup-go@v5.

@v4 is no longer supported and the runner rejects it, so the job will fail until we move to @v5.

-      uses: actions/setup-go@v4
+      uses: actions/setup-go@v5

23-23: Upgrade the cache action to @v4.

actions/cache@v3 is deprecated on GitHub-hosted runners; staying on it breaks the workflow, so we need to switch to @v4.

-      uses: actions/cache@v3
+      uses: actions/cache@v4
🧹 Nitpick comments (1)
docs/Gemfile (1)

7-9: Order the plugin gems alphabetically.

Bundler’s OrderedGems check flags this block: please list jekyll-feed, jekyll-seo-tag, then jekyll-sitemap so the plugins are sorted alphabetically and lint stays green.

   gem "jekyll-feed", "~> 0.12"
-  gem "jekyll-sitemap"
-  gem "jekyll-seo-tag"
+  gem "jekyll-seo-tag"
+  gem "jekyll-sitemap"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd9e21e and 7894c5f.

📒 Files selected for processing (13)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/pages.yml (1 hunks)
  • docs/Gemfile (1 hunks)
  • docs/README.md (1 hunks)
  • docs/_config.yml (1 hunks)
  • docs/_layouts/default.html (1 hunks)
  • docs/_layouts/exercise.html (1 hunks)
  • docs/about.md (1 hunks)
  • docs/assets/css/gopher-theme.css (1 hunks)
  • docs/contributing.md (1 hunks)
  • docs/exercises.md (1 hunks)
  • docs/getting-started.md (1 hunks)
  • docs/index.md (1 hunks)
✅ Files skipped from review due to trivial changes (5)
  • docs/index.md
  • docs/exercises.md
  • docs/README.md
  • docs/about.md
  • docs/getting-started.md
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml

18-18: the runner of "actions/setup-go@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


23-23: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 RuboCop (1.81.1)
docs/Gemfile

[convention] 9-9: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem jekyll-seo-tag should appear before jekyll-sitemap.

(Bundler/OrderedGems)

Comment thread docs/_config.yml

# Social links (optional)
social:
github: "your-username/golearn"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix the GitHub repo slug so generated links work.

Everything that pulls site.social.github (e.g., the “View Template/Solution” buttons in the exercise layout and the footer link) points to https://github.com/your-username/golearn, which 404s. Please replace this placeholder with the actual repository slug (e.g., zhravan/golearn) so published docs link to the right code.

-social:
-  github: "your-username/golearn"
+social:
+  github: "zhravan/golearn"
🤖 Prompt for AI Agents
In docs/_config.yml around line 64, the github field currently uses the
placeholder "your-username/golearn" which produces broken links; replace that
value with the actual GitHub repository slug (for example "zhravan/golearn") so
all generated links that use site.social.github point to the correct repository.

@zhravan zhravan added patch Bug fixes and small improvements hacktoberfest Hacktoberfest participation hacktoberfest-accepted hacktoberfest2025 labels Oct 1, 2025
@zhravan
zhravan merged commit 9ca947d into main Oct 1, 2025
1 check passed
@zhravan
zhravan deleted the trigger-b branch October 1, 2025 23:49
This was referenced Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest Hacktoberfest participation hacktoberfest2025 hacktoberfest-accepted patch Bug fixes and small improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant